Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #245: return error for NaN in naive bayes #246

Open
wants to merge 7 commits into
base: development
Choose a base branch
from
Open

Conversation

Mec-iS
Copy link
Collaborator

@Mec-iS Mec-iS commented Nov 21, 2022

Fix #245

Check if log_likehood is NaN and return an error as a Result. Add test as suggested.

PS. the test suggested is in conflict with the original expected behaviour (see tests in mod.rs) so it couldn't be implemented. A better handling of NaN has been implemented instead, following the original behaviour.

@Mec-iS Mec-iS requested a review from morenol November 21, 2022 11:34
@Mec-iS
Copy link
Collaborator Author

Mec-iS commented Nov 21, 2022

cc: @chriamue

@codecov-commenter
Copy link

Codecov Report

Merging #246 (f2f4068) into development (d15ea43) will increase coverage by 0.13%.
The diff coverage is 63.63%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@               Coverage Diff               @@
##           development     #246      +/-   ##
===============================================
+ Coverage        44.50%   44.64%   +0.13%     
===============================================
  Files               85       85              
  Lines             7226     7235       +9     
===============================================
+ Hits              3216     3230      +14     
+ Misses            4010     4005       -5     
Impacted Files Coverage Δ
src/naive_bayes/gaussian.rs 36.45% <ø> (+1.04%) ⬆️
src/naive_bayes/mod.rs 44.11% <63.63%> (+12.11%) ⬆️

... and 8 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@smartcorelib smartcorelib deleted a comment from grinapo Jan 22, 2025
* general behaviour has been kept unchanged according to original tests in `mod.rs`
* aka: error is returned only if all the predicted probabilities are NaN
@Mec-iS Mec-iS requested a review from morenol January 22, 2025 13:31
@Mec-iS
Copy link
Collaborator Author

Mec-iS commented Jan 22, 2025

@chriamue please review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Prediction of GaussianNB panics in some conditions
3 participants